Fix 'Not Found' page flash after deleting expense#83613
Fix 'Not Found' page flash after deleting expense#83613
Conversation
The deleteTransactionNavigateBackUrl clearing effect in DeleteTransactionNavigateBackHandler had an inverted focus condition compared to SearchMoneyRequestReportPage. It cleared the guard URL while the screen was still focused, allowing the Not Found page to briefly flash before navigation completed. Changed the condition to only clear after navigating away, matching the established pattern. Co-authored-by: Krishna <Krishna2323@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
@MelvinBot The issue is not fixed. While keeping the updates seems reasonable to me, the root cause appears to be a timing issue. This only occurs on slower devices, where it takes some time for the RHP/Details page to close after the report is deleted. During that transition, the “Not Found” page becomes briefly visible while the RHP is closing. |
|
Thanks for the feedback Root CauseThe if (shouldShowNotFoundPage && contentShown.current && !isFocused) {
return null; // prevents NotFound during close animation
}This guard only returns
Precedent in the CodebaseThis exact pattern is already handled for transaction duplicate reviews in // Delay clearing review duplicate data till the RHP is completely closed
// to avoid not found showing briefly in confirmation page when RHP is closing
InteractionManager.runAfterInteractions(() => {
abandonReviewDuplicateTransactions();
});Suggested FixThe Navigation.setNavigationActionToMicrotaskQueue(() => {
navigateToTargetUrl();
// Delay deletion until RHP close animation finishes
InteractionManager.runAfterInteractions(() => {
deleteTransaction();
});
});This ensures the report data stays intact while the RHP is still visible and animating closed. The condition fix in this PR (clearing the guard when Want me to update this PR with the |
@MelvinBot yes please. |
Wraps deleteTransaction() in InteractionManager.runAfterInteractions() so that Onyx data isn't cleared while the RHP panel is still visibly animating closed. This prevents the withReportOrNotFound HOC from rendering a brief "Not Found" flash on slower devices where the isFocused state hasn't flipped to false yet during the animation. Co-authored-by: Krishna <Krishna2323@users.noreply.github.com>
|
Updated the PR with the 1. 2. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@MelvinBot move this out of draft |
|
Done — PR has been moved out of draft and is now ready for review. |
|
@Krishna2323 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@MelvinBot please update the PR description to:
|
|
Updated the PR description with:
|
Explanation of Change
After deleting an expense from the report details page (open expense → click report details header → delete), a "Not Found" page briefly flashes before the user is navigated away. This is a timing issue most visible on slower devices, where the RHP close animation takes longer to complete.
This PR makes two changes:
1. Defer
deleteTransaction()inReportDetailsPage.tsxuntil after the RHP close animation finishesWhen the user confirms deletion from
ReportDetailsPage, both navigation (dismiss the RHP) and data deletion happen in the samerequestAnimationFramecallback. On slower devices, the Onyx update from deletion propagates before the RHP has finished its 300ms slide-out animation. This causes thewithReportOrNotFoundHOC — still rendered inside the animating-out RHP — to briefly show a "Not Found" page.The fix wraps
deleteTransaction()inInteractionManager.runAfterInteractions()so the Onyx data is only cleared after the close animation completes. This matches the exact same pattern already used elsewhere in the codebase for the same reason:MoneyReportHeader.tsx:1539-1544— deletes transactions after interactions to "not show the not found page before navigating to goBackRoute"MoneyReportHeader.tsx:1563-1569— defersdeleteAppReportuntil aftergoBackanimation completesRightModalNavigator.tsx:130-135— delays clearing duplicate review data "till the RHP is completely closed to avoid not found showing briefly"InteractionManager.runAfterInteractionsis semantically the best fit here — it is designed exactly to defer work until animations and interactions are complete, which is precisely the problem being solved (data mutation during a close animation).Note on the
@typescript-eslint/no-deprecatedsuppression: The deprecation ofInteractionManager.runAfterInteractionsis only a TypeScript-level annotation from the React Native types. The API still works and is widely used throughout this codebase (see links above). The eslint-disable comment matches the existing pattern used at every other call site.2. Fix inverted focus condition in
DeleteTransactionNavigateBackHandler.tsxThe
deleteTransactionNavigateBackUrlguard was being cleared whileReportScreenwas still focused (!isFocused), allowingshouldShowNotFoundPageto briefly evaluate totruebefore navigation completed. This fix changes the condition toisFocused(matching the correct pattern established inSearchMoneyRequestReportPage.tsx:117-123), so the guard URL is only cleared after the user has navigated away from the screen.Fixed Issues
$ #83521
PROPOSAL: #83521 (comment)
Tests
Slow device test:
Regression test (forward button behavior from original PR #79530):
Offline tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Not applicable - logic-only change, no UI modification.
Android: mWeb Chrome
Not applicable - logic-only change, no UI modification.
iOS: Native
Not applicable - logic-only change, no UI modification.
iOS: mWeb Safari
Not applicable - logic-only change, no UI modification.
MacOS: Chrome / Safari
Not applicable - logic-only change, no UI modification.